Skip to content

Keep Charts preview history in sync - #1179

Merged
tannerlinsley merged 1 commit into
mainfrom
agent/sync-charts-preview-history
Aug 26, 2026
Merged

Keep Charts preview history in sync#1179
tannerlinsley merged 1 commit into
mainfrom
agent/sync-charts-preview-history

Conversation

@tannerlinsley

@tannerlinsley tannerlinsley commented Aug 25, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Evidence

Both Charts preview message handlers read previewHistoryRef.current, but updated that ref inside a React state updater. State updaters can be evaluated after the message handler returns, so a second browser-state message can read stale history and calculate navigation or annotation state from the wrong URL. The Builder instance was also left as an unresolved review finding on merged PR #1171.

The same pattern exists in the catalog result handler. No open issue or PR addresses the synchronization bug. PR #1173 touches the catalog result only to remove an unused callback, not preview history behavior.

Impact

Rapid preview navigation messages now build on the latest accepted history in both Charts surfaces. This prevents stale back and forward entries and annotation targets.

Change

Compute the next history from the current ref, update the ref synchronously, then pass that exact value to React state. This keeps the existing history helper and behavior, with no new abstraction or API change.

The branch is rebased on current main after the Charts Notebook page was renamed to Charts Builder.

Validation

  • pnpm test
  • TypeScript and type-aware lint passed
  • 466 tests total, 465 passed, 1 environment-gated docs smoke test skipped
  • git diff --check

Risk

Low. The same pure updateExamplePreviewHistory result still drives state, but the mutable ref now advances in the message handler before another message can arrive.

Summary by CodeRabbit

  • Bug Fixes
    • Improved browser navigation and preview history updates for more reliable back-and-forward behavior.
    • Preserved URL normalization, navigation-error clearing, and annotation target resets during history changes.
    • Improved synchronization between sandbox navigation state and the displayed preview.

@tannerlinsley tannerlinsley added the source-audit Tracked by the automated source audit label Aug 25, 2026
@coderabbitai

coderabbitai Bot commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5836c213-1771-4acb-b03a-f1a1a0723eab

📥 Commits

Reviewing files that changed from the base of the PR and between 302e8e3 and f2ec298.

📒 Files selected for processing (1)
  • src/components/charts/ChartsBuilderPage.client.tsx

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.

📝 Walkthrough

Walkthrough

The chart catalog and builder now derive preview history from the current ref, update the ref immediately, and then update React state with the same history snapshot.

Changes

Preview history synchronization

Layer / File(s) Summary
Synchronize preview history snapshots
src/components/charts/ChartsCatalogResult.client.tsx, src/components/charts/ChartsBuilderPage.client.tsx		 Browser-state handlers compute nextHistory from previewHistoryRef.current, synchronize the ref, and call setPreviewHistory(nextHistory).

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f2ec2

The change synchronously keeps preview history aligned with rapid navigation messages without changing the existing history behavior or API. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: synchronizing Charts preview history across the affected surfaces.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/sync-charts-preview-history

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs		 tanstack-com f2ec298 Commit Preview URL

Branch Preview URL		 Aug 26 2026, 03:36 PM

@tannerlinsley
tannerlinsley force-pushed the agent/sync-charts-preview-history branch from 302e8e3 to f2ec298 Compare August 26, 2026 15:34
@tannerlinsley
tannerlinsley merged commit 2c522a5 into main Aug 26, 2026
4 checks passed
@tannerlinsley
tannerlinsley deleted the agent/sync-charts-preview-history branch August 26, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

source-audit Tracked by the automated source audit

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tannerlinsley